Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Navigation Block: Remove obsolete Block Hooks filters #64676

Merged
merged 3 commits into from
Nov 20, 2024
Merged

Navigation Block: Remove obsolete Block Hooks filters #64676

merged 3 commits into from
Nov 20, 2024
+0 −298

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Aug 21, 2024 •
edited
Loading

What?

Remove a number of Block Hooks related filters from the Navigation block.

Why?

As of WP 6.6, Core takes care of these things (by adding its own filters):

  • block_core_navigation_set_ignored_hooked_blocks_metadata was a helper that was only used by block_core_navigation_update_ignore_hooked_blocks_meta.
  • block_core_navigation_update_ignore_hooked_blocks_meta was replaced by update_ignored_hooked_blocks_postmeta, which is added as a default filter here.
  • block_core_navigation_insert_hooked_blocks_into_rest_response was replaced by insert_hooked_blocks_into_rest_response, which is added as a default filter here.

And per #67117, 6.6 is the minimum required WordPress version for the Gutenberg plugin.

How?

By removing code.

Note

There's a companion PR in wordpress-develop to move these functions into deprecated.php: WordPress/wordpress-develop#7849

Testing Instructions

This should be covered by unit tests (which are run both against the current and previous WP versions).
In addition, you can smoke-test Block Hooks in the Navigation block as described in other related PRs, e.g. #59021.

@ockham ockham added [Status] Blocked Used to indicate that a current effort isn't able to move forward [Type] Code Quality Issues or PRs that relate to code quality [Block] Navigation Affects the Navigation Block [Feature] Block hooks labels Aug 21, 2024
@ockham ockham self-assigned this Aug 21, 2024
@ockham ockham changed the base branch from trunk to remove/now-obsolete-block-hooks-compat-checks August 21, 2024 09:13
@ockham ockham mentioned this pull request Aug 21, 2024
Merged
Base automatically changed from remove/now-obsolete-block-hooks-compat-checks to trunk August 21, 2024 14:01
@ockham ockham force-pushed the remove/navigation-block-block-hooks-filters branch from 47d632c to d4a6878 Compare August 21, 2024 14:01
@ockham ockham force-pushed the remove/navigation-block-block-hooks-filters branch from d4a6878 to fed8038 Compare September 26, 2024 06:33
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 26, 2024 •
edited
Loading

Flaky tests detected in 6bc486d.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11933328108
📝 Reported issues:

@ockham
Copy link
Contributor Author

ockham commented Sep 30, 2024

This might require a counterpart PR in Core that moves the functions we're removing here into src/wp-includes/deprecated.php 🙈

@ockham ockham force-pushed the remove/navigation-block-block-hooks-filters branch from fed8038 to 779de94 Compare November 12, 2024 13:17
@ockham ockham mentioned this pull request Nov 19, 2024
Merged
@ockham ockham force-pushed the remove/navigation-block-block-hooks-filters branch from 779de94 to a49161f Compare November 20, 2024 11:54
@ockham ockham changed the title Navigation Block: Remove obsolete Block Hooks filters [AFTER WP 6.7!] Navigation Block: Remove obsolete Block Hooks filters Nov 20, 2024
@ockham ockham mentioned this pull request Nov 20, 2024
Draft
@ockham ockham marked this pull request as ready for review November 20, 2024 14:22
@ockham ockham requested a review from ajitbohra as a code owner November 20, 2024 14:22
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 20, 2024 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ockham <[email protected]>
Co-authored-by: Mamaduka <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ockham ockham requested review from a team, mirka and richtabor and removed request for a team November 20, 2024 14:22
@ockham ockham merged commit d57502e into trunk Nov 20, 2024
70 checks passed
@ockham ockham deleted the remove/navigation-block-block-hooks-filters branch November 20, 2024 18:57
@github-actions github-actions bot added this to the Gutenberg 19.8 milestone Nov 20, 2024
@ockham ockham mentioned this pull request Nov 21, 2024
Merged
@ockham ockham removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mamaduka Mamaduka Mamaduka approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@mirka mirka Awaiting requested review from mirka mirka was automatically assigned from WordPress/gutenberg-core

@richtabor richtabor Awaiting requested review from richtabor richtabor was automatically assigned from WordPress/gutenberg-core

Assignees

@ockham ockham

Labels
[Block] Navigation Affects the Navigation Block [Feature] Block hooks [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Milestone
Gutenberg 19.8
Development

Successfully merging this pull request may close these issues.
2 participants
@ockham @Mamaduka